Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
5d4ec81 to
61851cb
Compare
|
🔔 1st Reminder Hey @wpaulino! This PR has been waiting for your review. |
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 4th Reminder Hey @tnull! This PR has been waiting for your review. |
tnull
left a comment
There was a problem hiding this comment.
Thank you! Pretty much looks good to me!
One minor nit: please rewrite your commit messages to wrap at 72 characters. In general, feel free to consult https://cbea.ms/git-commit/ for guidance on how to write good commit messages.
docker-compose.yml
Outdated
|
|
||
| bitcoin: | ||
| container_name: ldk-server-bitcoin | ||
| image: blockstream/bitcoind:29.1@sha256:9fcffa83feed0fc382dfb2f26990ca07656d150ba49434096a5aeedac6695a01 |
There was a problem hiding this comment.
Would be cool to go for v30 directly here, just pinged somebody from Blockstream to check if they intend to upgrade the Docker image soon.
There was a problem hiding this comment.
Sounds good, let’s wait for their feedback then.
There was a problem hiding this comment.
And there it is: https://hub.docker.com/r/blockstream/bitcoind
Please update!
61851cb to
ccdefcd
Compare
ccdefcd to
d573de7
Compare
| args: | ||
| BUILD_FEATURES: "" | ||
| command: [ | ||
| "--node-listening-address=0.0.0.0:3001", |
There was a problem hiding this comment.
seems weird for ldk-server we use the non-default 3001 but give the default 9735 to lnd
There was a problem hiding this comment.
LND uses port 9735, and LDK uses port 3001. I checked in my tests, and LDK is indeed using 3001.
| COPY --from=builder /opt/app/ldk-server/ldk-server-config.toml /usr/local/bin/ldk-server-config.toml | ||
| RUN chmod +x /usr/local/bin/ldk-server | ||
|
|
||
| EXPOSE 3000 3001 |
There was a problem hiding this comment.
This port will conflict with LND.
There was a problem hiding this comment.
the dockerfile doesn't contain lnd, only ldk-server
There was a problem hiding this comment.
Yeah, but the docker-compose contains LND.
There was a problem hiding this comment.
The docker-compose here will just be an example regtest lightning network people will set up for testing, but the Dockerfile is actually how people will deploy production nodes so we should be optimizing for that.
Dockerfile
Outdated
|
|
||
| RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | ||
| ENV PATH="/root/.cargo/bin:${PATH}" | ||
| RUN rustup default 1.90.0 |
There was a problem hiding this comment.
why use a debian container if you're just going to install rustup into it? why not just use the rust container
There was a problem hiding this comment.
Makes sense, I replaced Debian with Rustup instead.
Dockerfile
Outdated
| COPY --from=builder /opt/app/target/release/ldk-server /usr/local/bin/ldk-server | ||
| COPY --from=builder /opt/app/target/release/ldk-server-cli /usr/local/bin/ldk-server-cli | ||
| COPY --from=builder /opt/app/ldk-server/ldk-server-config.toml /usr/local/bin/ldk-server-config.toml | ||
| RUN chmod +x /usr/local/bin/ldk-server |
There was a problem hiding this comment.
is this needed? if it is should chmod the cli too
There was a problem hiding this comment.
This is not needed, I removed it.
Implements multi-stage build to produce a small final image by discarding build dependencies. Supports optional BUILD_FEATURES for additional features, with cached compilation in builder stage.
introduces docker-compose.yml with Bitcoin Core, CLN, and LND for Lightning testing. The default profile runs these services; the 'ldk-server' profile adds the LDK server; the 'rabbitmq' profile includes RabbitMQ for events.
d573de7 to
2cb3857
Compare
|
Sorry for the delay, I’m still attending Bitcoin events. |
|
I'm closing this PR because I don't have much time to continue it. |
Adds Docker support for building and testing LDK Server. Includes a multi-stage Dockerfile for optimized builds and a docker-compose.yml for a full testing environment with Bitcoin Core, CLN, and LND.
BUILD_FEATURES) and final stage for runtime. Exposes ports 3000 and 3001 for REST and Lightning connectivity.docker compose up).docker compose --profile ldk-server up).docker compose --profile rabbitmq up).this pr depends on #69